fix(authz): protect purpose-bound runtime integrity - #65

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

@opencode-agent Please review the current unchanged head against protected develop. Local exact-head verification: all owned package suites pass at 100% statement/branch coverage.

@coderabbitai review

Please review exact head c35d434adfdee94fa838819bba51bd2449ce940d. Focus on AuthorizationDecision construction-time runtime integrity, allow/deny field invariants, reference namespace binding, compatibility with existing direct decision constructors, and the new adversarial decision-evidence regressions. Do not transfer findings or approval from predecessor heads.

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Exact revision =='
git rev-parse HEAD
git status --short
printf '%s\n' '== Merge base with develop =='
git merge-base HEAD develop
printf '%s\n' '== Changed files =='
git diff --name-status develop...HEAD
printf '%s\n' '== Python structure candidates =='
fd -e py . | xargs -r ast-grep outline --items all --type class,function --match 'AuthorizationDecision|Authorization|Decision' 2>/dev/null || true
printf '%s\n' '== Focused diff =='
git diff --find-renames --unified=80 develop...HEAD -- '*.py'

#!/bin/bash
set -euo pipefail

printf '%s\n' '== Authorization module, complete numbered source =='
cat -n packages/keyverse-adapter/src/orgmetra_keyverse_adapter/authorization.py

printf '%s\n' '== Direct constructors and consumer references =='
rg -n -C 8 --glob '*.py' \
  '(AuthorizationDecision\s*\(|\bAuthorizationDecision\b)' \
  packages services

printf '%s\n' '== Existing adapter authorization tests =='
cat -n packages/keyverse-adapter/tests/test_authorization.py

printf '%s\n' '== Persistence-boundary authorization validation =='
sed -n '150,230p' services/people-api/src/orgmetra_people_api/postgres_hire.py
sed -n '1,260p' services/people-api/src/orgmetra_people_api/postgres_mutations.py

printf '%s\n' '== Decision construction test map =='
rg -n -C 5 --glob 'test_*.py' \
  '(AuthorizationDecision\s*\(|_decision\s*\(|allowed=False|authorized_fields)' \
  packages/keyverse-adapter/tests services/people-api/tests services/job-analysis-api/tests

@seonghobae I found one runtime-integrity bypass on exact head c35d434adfdee94fa838819bba51bd2449ce940d.

High: Prevent AuthorizationDecision subclass validation bypass

AuthorizationDecision is still subclassable. A subclass can override __post_init__ and omit super().__post_init__(). The generated dataclass initializer dispatches to that override. The subclass can then contain malformed or executable evidence.

The People persistence boundaries accept the subclass because they use isinstance(authorization, AuthorizationDecision).

• packages/keyverse-adapter/src/orgmetra_keyverse_adapter/authorization.py, Line 188
• services/people-api/src/orgmetra_people_api/postgres_hire.py, Line 179
• services/people-api/src/orgmetra_people_api/postgres_mutations.py, Line 325

This bypass can reintroduce the forged allowed, UUID, string, field-set, and reference values that this change intends to reject.

Make AuthorizationDecision non-subclassable, for example with an __init_subclass__ method that raises TypeError. Add an adversarial regression that defines a subclass which skips __post_init__, then verify that class definition or construction fails. This preserves existing direct construction of the exact AuthorizationDecision class.

I did not transfer predecessor findings or approval. I completed static review only. The sandbox cannot execute the repository test suites.

@coderabbitai review

Please review exact head edbd07337162f85ac7ab6b74e570b49e53a6d296, focusing on AuthorizationDecision verdict/reason polarity, preservation of extensible denial codes and non-authoritative bounded next_action, and compatibility with direct People persistence/replay fixtures. Do not transfer predecessor findings or GREEN evidence.

Owner-path update after the #268 source repair: canonical #64 is now exact 6c8be6d44e28fe77bfe52a2e0946df4e0cbe3b83. #268 adds test-first retained nested-UUID mutation coverage for Employment, Position and Assignment across executable purpose-bound authorization, plus a direct PostgreSQL Position connection-factory callback regression; the production repair reconstructs every validated generic-command UUID from its once-read exact integer payload, so existing dataclasses.replace(...) boundaries detach nested identity authority as well as the outer command. Current #64 exact-head Foundation/Security/SAST/CodeQL runs are still queued and no qualifying APPROVED review exists, so this is not protected truth yet. #65 must not copy the mutable source; after #64 integrates normally, non-force adopt protected #64 including #262/#264/#266/#267/#268, then reconcile #65's own detached/revalidated AuthorizationDecision semantics and reacquire exact-head gates/review.

Owner-order update: canonical #64 has advanced by ordinary forward commits to 41829e0971a79012a2cd3723b8c355baa5acac3c on the same protected develop@eb9757f... base. In addition to #267/#268 nested command UUID detachment, #269 now requires accepted People mutation results themselves to be detached from adapter-retained outer result aliases and nested UUID payloads before validation/use/return. #65 must not copy mutable #64 source. After #64 normally integrates, non-force adopt protected truth and preserve #229–#233, #246, packaging/runtime-floor repairs, #262, #264, #266, #267, #268, #269 plus #65's own detached/revalidated AuthorizationDecision semantics. Current #64 workflows for 41829e... are newly queued/pending; no predecessor GREEN transfer.

Fresh canonical #64 owner handoff: #270 adds a separate post-executable replay-verification invariant after #246/#269. The replay check must not reuse either a semantic command object or an AuthorizationDecision object after those same objects have crossed an executable mutation-port boundary; otherwise the port can mutate checked evidence and manufacture a digest that the application verifies against the changed state. Current #64 keeps a second detached command copy for the port and reduces the authorized command + decision to an inert expected replay digest before persistence. After #64 normally integrates, #65 should non-force adopt protected truth and preserve #270 together with #229–#233/#246/#262/#264/#266–#269 and its own broader detached/revalidated AuthorizationDecision semantics. Do not copy current mutable #64 source or transfer predecessor checks.

Owner-path update after #271 revalidation: #64 ordinary-forward head 19c6c7e... removed the invalidated Employment-root lock experiment. New #272 is the canonical #64 acceptance gap for real PostgreSQL two-session Assignment concurrency evidence; #65 must inherit that protected test contract after #64 normal integration, but must not copy mutable #64 source or reintroduce #271. #65's own detached/revalidated AuthorizationDecision semantics remain unchanged.

#272 downstream handoff correction: when #65 eventually non-force adopts protected #64, preserve the PostgreSQL concurrency behavioral contract and evidence-tier boundary, but do not copy the current mutable test harness. Fresh review of #64 exact 1ea5cb1d... found that _exercise_conflict(...) lacks finally cleanup around DB lock observation; an assertion failure can leave writer A blocked at its pre-COMMIT barrier while the ephemeral PostgreSQL container unwinds. #272 now owns deterministic release/rollback + thread-join cleanup before teardown.

Also preserve the corrected evidence classification: deterministic synthetic structural rows may establish PostgreSQL mechanism/integration behavior, but they are not real/right-cleared buyer/scientific acceptance data. Do not inherit the superseded #271 Employment lock.

#64 owner handoff — fresh exact source is now 558afdf482af7e6e69ea0694e9f731365f1300be on protected develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f direct base. Intervening 6cf31e... repaired #272 failure cleanup by releasing the pre-COMMIT barrier and joining every started writer from a finally. 558afdf... adds the missing executable preservation regression: observe the real PostgreSQL lock first, force an assertion failure immediately afterward, then require zero concurrency-writer sessions in pg_stat_activity while the container is still alive. Do not copy this mutable harness/source. After #64 is normally integrated, non-force adopt only the protected behavioral contract: real DB-visible conflict serialization, failure-path writer cleanup before teardown, synthetic-integration-vs-real/right-cleared evidence separation, plus #64's retained #229–#233/#246/#262/#264/#266–#270 invariants. #65 remains owner of detached/revalidated AuthorizationDecision semantics.

#64 / #272 owner-path handoff update: current #64 exact head b71263918cc59b9ca4177abf1770f1c87b619440 extends the Assignment PostgreSQL failure-cleanup contract. A bounded Thread.join(timeout=30) is not sufficient evidence of quiescence: if the writer is still alive and its captured libpq connection remains open, #64 now targets only the verified owned backend (captured PID + internal writer application_name) with pg_terminate_backend, then performs a second bounded join and fails closed if the thread remains live. Do not copy the mutable harness/source into #65. After #64 is normally integrated into protected develop, non-force adopt the behavioral contract together with #272's DB-visible serialization and synthetic-vs-real evidence boundary, then preserve #65's own detached/revalidated AuthorizationDecision semantics.